Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: enable feature tagging #2395

Merged
merged 2 commits into from
Dec 3, 2024
Merged

docs: enable feature tagging #2395

merged 2 commits into from
Dec 3, 2024

Conversation

toidiu
Copy link
Contributor

@toidiu toidiu commented Dec 3, 2024

Resolved issues:

#1831

Description of changes:

This PR enables feature tagging in our rust docs. This is useful for customers that want to use enable APIs that require building s2n-quic with certain features.

Call-outs:

This PR only covers the main s2n-quic crate. Future work might be to expand feature tagging to other s2n-quic crates.

We need to gate the feature on docsrs since it requires nightly cargo and would otherwise break stable builds.

We rely doc_auto_cfg to auto generate the feature tagging where applicable. However, we also require manually tagging with doc_cfg

  • since auto isn't able to detect the use of cfg_if! macro
  • for the tls providers since they key on the crate name

Testing:

The example report in CI shows the feature tagging.

I also tested this on a personal test crate to verify crates.io works as intended.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@toidiu toidiu requested a review from camshaft December 3, 2024 21:06
@toidiu toidiu marked this pull request as ready for review December 3, 2024 21:06
# Compile with the `s2n_docsrs` feature to enable feature tagging in rust docs. See
# documentation for additional configuration options: https://docs.rs/about/metadata
[package.metadata.docs.rs]
rustdoc-args = ["--cfg", "s2n_docsrs"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we need our own? Why not just use the one docs.rs already sets?

https://github.com/tokio-rs/tokio/blob/dcae2b9eb8f32eb92ecb3efe126b91577c5096e4/tokio/src/lib.rs#L637

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea I wasn't sure if that was something that docsrs set or if tokio didnt namespace the cfg flag. Lemme try and find docs mentioning that and/or test it on my test crate

Copy link
Contributor Author

@toidiu toidiu Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So actually are you certain that gets set by docsrs? Tokio also sets rustdoc-args which specifies docsrs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing shows that docsrs and that we dont need package.metadata.docs.rs either.

@toidiu toidiu requested a review from camshaft December 3, 2024 22:13
Copy link
Contributor

@camshaft camshaft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thanks for doing this :)

@camshaft camshaft merged commit 82943e4 into main Dec 3, 2024
128 of 130 checks passed
@camshaft camshaft deleted the ak-docsrs2 branch December 3, 2024 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants